Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose user blocking in the admin UI #4363

Merged
merged 8 commits into from Apr 6, 2020

Conversation

plourenco
Copy link
Member

@plourenco plourenco commented Mar 19, 2020

Closes #3243

This PR will expose the endpoint and UI ability to block/unblock users, and:

  • Show that a user account is blocked when viewing their profile and maybe also in the user search results
  • Filter out blocked users when searching in non-admin areas - RFC

image

image

image

Side-effects

Since this commit used some semantic-ui raw elements, I took the opportunity to upgrade most of the previous custom CSS within the user profile section. These include, but not limited to, toggles, buttons and header panels (alignment refactoring only).

image

indico/modules/users/blueprint.py Outdated Show resolved Hide resolved
indico/modules/users/controllers.py Outdated Show resolved Hide resolved
indico/modules/users/controllers.py Outdated Show resolved Hide resolved
indico/modules/users/templates/personal_data.html Outdated Show resolved Hide resolved
indico/modules/users/templates/personal_data.html Outdated Show resolved Hide resolved
indico/modules/users/templates/personal_data.html Outdated Show resolved Hide resolved
@plourenco plourenco force-pushed the ui-user-blocking branch 2 times, most recently from e27fc7e to 11c5910 Compare March 23, 2020 12:37
@plourenco
Copy link
Member Author

Updated my last commit to include labels in the user dashboard.
image

@plourenco plourenco force-pushed the ui-user-blocking branch 2 times, most recently from a513935 to d9453db Compare March 23, 2020 12:59
@plourenco plourenco changed the title WIP: Expose user blocking in the admin UI Expose user blocking in the admin UI Mar 23, 2020
indico/modules/users/templates/dashboard.html Outdated Show resolved Hide resolved
indico/modules/users/controllers.py Outdated Show resolved Hide resolved
indico/modules/users/templates/users_admin.html Outdated Show resolved Hide resolved
@mic4ael
Copy link
Member

mic4ael commented Mar 24, 2020

Just noticed a small thing which might not be related to your additions but I think it is something worth looking into.

The elements in each line seem to be misaligned.

Screenshot 2020-03-24 at 10 22 17

@plourenco
Copy link
Member Author

plourenco commented Mar 24, 2020

Just noticed a small thing which might not be related to your additions but I think it is something worth looking into.

The elements in each line seem to be misaligned.

Screenshot 2020-03-24 at 10 22 17

It is related, yes. The new semantic-ui buttons are a bit bigger.
Nice catch.

@ThiefMaster
Copy link
Member

Maybe we can use a smaller size for the button there?

indico/modules/api/templates/user_profile.html Outdated Show resolved Hide resolved
indico/modules/users/templates/emails.html Outdated Show resolved Hide resolved
indico/modules/api/templates/user_profile.html Outdated Show resolved Hide resolved
indico/web/client/styles/base/_typography.scss Outdated Show resolved Hide resolved
indico/modules/users/templates/dashboard.html Outdated Show resolved Hide resolved
indico/modules/users/controllers.py Outdated Show resolved Hide resolved
indico/modules/users/controllers.py Outdated Show resolved Hide resolved
indico/modules/users/templates/users_admin.html Outdated Show resolved Hide resolved
indico/web/client/styles/partials/_boxes.scss Outdated Show resolved Hide resolved
class RHUserBlock(RHUserBase):
def _process_PUT(self):
self.user.is_blocked = True
logger.info("User %s blocked %s", session.user, self.user)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for being nitpicky but we usually use single quotes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem! I thought the linter was picking this up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does flake8 have a plugin which can check for this (single quotes, unless it would require escaping inside)? But even if it does, I think it'd require quite a few changes in existing files for English strings where we defaulted to double quotes for a while even if there were no apostrophes inside.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice, this looks great! I just gave it a try and with some configuration it works pretty well to avoid stuff like unnecessary escapes.

unfortunately the "wrong type of quotes" check isn't really suitable for us:

$ flake8 indico/ | grep Q000 | wc -l
1366

a bit better for multiline strings, but that's more because we don't have that many which aren't docstrings..

$ flake8 indico/ | grep Q001 | wc -l
138

and even better when ignoring it for indico/core/signals/ since the multiline strings there are basically docstrings (and thus double quotes are more appropriate).

$ flake8 indico/ | grep Q001 | wc -l
34

But I don't think it really matters for multiline strings, so I'm inclined to disable Q000 and Q001 globally and just let it complain about unnecessary escapes and docstrings not using double-quotes.

Too bad there's no way to run this check just for newly added code...

(and no, I don't want to update those 1.3k occurrences of double quotes, even if there's probably some tool that can automate it)

indico/modules/users/controllers.py Outdated Show resolved Hide resolved
indico/modules/users/templates/_favorites.html Outdated Show resolved Hide resolved
indico/modules/users/templates/emails.html Outdated Show resolved Hide resolved
@plourenco plourenco force-pushed the ui-user-blocking branch 2 times, most recently from c5e9c7f to e0c461f Compare March 30, 2020 13:48
@@ -402,7 +402,7 @@ def _process(self):
external = form_data.pop('external')
form_data = {k: v for (k, v) in form_data.iteritems() if v and v.strip()}
matches = search_users(exact=exact, include_deleted=include_deleted, include_pending=include_pending,
external=external, allow_system_user=True, **form_data)
include_blocked=True, external=external, allow_system_user=True, **form_data)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the admin search, I'm just including the blocked users by default because we already have way too many options and a label will be shown anyway. Nonetheless, I will leave this thread for change requests if anyone disagrees.

@plourenco plourenco force-pushed the ui-user-blocking branch 3 times, most recently from 5c4deb2 to 868e24d Compare April 2, 2020 21:07
Most of Indico's ui components use legacy custom CSS instead of the
semantic ui elements. Since this branch will introduce some new in the
user profile, it will also update this whole section elements for
consistency towards a gradual conversion.
This commit will add a new endpoint DELETE/PUT for user un/blocking and
exposes an ui button for the corresponding action.
Labels that reflect user properties were also added to the admin list.
The introduction of sui elements (mainly buttons) in the user profile
and the difference in their sizes caused the list items to lose their
alignment.

This commit refactors them to re-use previously declared flexbox css
styles.

Closes indico#4376.
@ThiefMaster ThiefMaster merged commit d980fc0 into indico:master Apr 6, 2020
@plourenco plourenco deleted the ui-user-blocking branch April 28, 2020 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose user blocking in the admin UI
4 participants